-
Notifications
You must be signed in to change notification settings - Fork 72
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix msearch_template Response #1178
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looking at the reference material (code + docs), this change seems wrong. What are the recordings that led to it?
@@ -65,9 +65,5 @@ export class MultiSearchResult<TDocument> { | |||
|
|||
/** @codegen_names result, failure */ | |||
export type ResponseItem<TDocument> = | |||
| MultiSearchItem<TDocument> | |||
| SearchResponse<TDocument> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We need MultiSearchItem
as multisearch response items have a status
field that indicates the result of each search request. This is present in the ES source code and explained in the docs.
@@ -73,7 +73,7 @@ export class ErrorResponseBase { | |||
// If the error is a string, it means that it was not caused by an exception on ES side, but on the HTTP routing layer. | |||
// This should never happen in clients, because we assume we will never send malformed request. | |||
error: ErrorCause | |||
status: integer | |||
status?: integer |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Status is always present in error responses, including in multisearch responses. See ES code and the docs "an object with error message and corresponding status code will be returned in place of the actual search response".
Keeping error
and status
required also plays an important role (in the Java client) to disambiguate some non-200 results where ES will return either a valid response or an error depending on the parameters (e.g. delete document that returns 404+error if the index doesn't exist but 404+valid response if the index exist but not the document).
Closing this in favour of #1320 |
as titled.